[PLUGIN-1824] ErrorDetailsProvider - MySql Source/Sink plugin #523

psainics · 2024-11-20T11:52:15Z

ErrorDetailsProvider - MySql [Source|Sink] plugin

Jira : PLUGIN-1824

Description

Implement Program Failure Exception Handling in MySql Source/Sink plugin to catch known errors

Code change

Added DBErrorDetailsProvider.java
Added MysqlErrorDetailsProvider.java
Modified DBRecord.java
Modified AbstractDBSink.java
Modified AbstractDBSource.java
Modified MysqlConstants.java
Modified MysqlSink.java
Modified MysqlSource.java
Modified MysqlUtil.java

itsankit-google · 2024-11-21T06:04:23Z

database-commons/src/main/java/io/cdap/plugin/db/source/AbstractDBSource.java

    if (sourceConfig.canConnect()) {
      try {
-        stageConfigurer.setOutputSchema(getSchema(driverClass));
+        stageConfigurer.setOutputSchema(getSchema(driverClass, collector));


we are already adding the exception in failure collector below by catching it, why is this extra step needed?

also can you please add sql state and error code in the message on line 127?

Added SQL error details.

itsankit-google · 2024-12-02T11:32:31Z

database-commons/src/main/java/io/cdap/plugin/db/DBErrorDetailsProvider.java

+    return ErrorUtils.getProgramFailureException(new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
+      errorMessage,
+      String.format(errorMessageFormat, errorContext.getPhase(), errorMessage), ErrorType.SYSTEM, false, e);
+


nit: remove empty line

itsankit-google

Can you please add open and capture logs step in the e2e tests where pipeline also fails so that we can verify the expected logs?

for example:

database-plugins/mysql-plugin/src/e2e-test/features/mysqlsource/RunTimeWithMacros.feature

Line 207 in 91d0ff7

    
           Scenario: Verify that pipeline fails when user provides invalid Credentials for connection with Macros

itsankit-google · 2024-12-02T11:41:18Z

database-commons/src/main/java/io/cdap/plugin/db/DBErrorDetailsProvider.java

+      if (t instanceof IllegalStateException) {
+        return getProgramFailureException((IllegalStateException) t, errorContext);
+      }
+


nit: remove empty line

itsankit-google · 2024-12-02T11:42:44Z

database-commons/src/main/java/io/cdap/plugin/db/DBErrorDetailsProvider.java

+   *
+   * @return The external documentation link as a {@link String}.
+   */
+  protected String getExternalDocumentationLink() {


where is this method used in this class?

This is how it is used in google-cloud plugin:

https://github.com/data-integrations/google-cloud/blob/92dc8b77e391935a28e3b3013cb51a11b85efbab/src/main/java/io/cdap/plugin/gcp/common/GCPErrorDetailsProvider.java#L86-L93

Oh ! This makes sense, I though this will be called by the platform and display on UI.

I have added the doc link in error details now !

itsankit-google · 2024-12-02T12:05:35Z

mysql-plugin/src/main/java/io/cdap/plugin/mysql/MysqlErrorDetailsProvider.java

+  protected ErrorType getErrorTypeFromErrorCode(int errorCode) {
+    // https://dev.mysql.com/doc/refman/9.0/en/error-message-elements.html#error-code-ranges
+    if (errorCode >= 1000 && errorCode <= 1999 || errorCode >= 2000 && errorCode <= 2999 ||
+      errorCode >= 3000 && errorCode <= 4999 || errorCode >= 5000 && errorCode <= 5999) {
+      return ErrorType.USER;
+    } else if (errorCode >= 10000 && errorCode <= 49999 || errorCode >= 50000 && errorCode <= 51999) {
+      return ErrorType.SYSTEM;
+    } else {
+      return ErrorType.UNKNOWN;
+    }
+  }
+}


It can be made more readable, something like this:

protected ErrorType getErrorTypeFromErrorCode(int errorCode) { // https://dev.mysql.com/doc/refman/9.0/en/error-message-elements.html#error-code-ranges // USER errors: General errors, server-specific, storage engine, and third-party engine errors if (errorCode >= 1000 && errorCode <= 5999) { return ErrorType.USER; } // SYSTEM errors: Enterprise and user-defined custom error messages else if (errorCode >= 10000 && errorCode <= 51999) { return ErrorType.SYSTEM; } // UNKNOWN errors: Anything outside defined ranges else { return ErrorType.UNKNOWN; } }

itsankit-google · 2024-12-04T06:04:16Z

mysql-plugin/src/e2e-test/features/mysqlsink/RunTimeWithMacros.feature

    And Run the Pipeline in Runtime with runtime arguments
    Then Wait till pipeline is in running state
    Then Open and capture logs
+    And Open and capture logs


this step looks duplicate.

Removed dupe !

itsankit-google · 2024-12-04T06:13:32Z

database-commons/src/main/java/io/cdap/plugin/db/sink/AbstractDBSink.java

    addOutputContext(context);
+
+    // set error details provider
+    context.setErrorDetailsProvider(new ErrorDetailsProviderSpec(getErrorDetailsProviderClassName()));


context.setErrorDetailsProvider should be called before addOutputContext(context) is called.

Otherwise error details provider information is not propagated to platform.

Understood, moved !

itsankit-google · 2024-12-04T06:14:34Z

database-commons/src/main/java/io/cdap/plugin/db/source/AbstractDBSource.java

      DataDrivenETLDBInputFormat.class, connectionConfigAccessor.getConfiguration())));
+
+    // set error details provider
+    context.setErrorDetailsProvider(new ErrorDetailsProviderSpec(getErrorDetailsProviderClassName()));


similarly here:

context.setErrorDetailsProvider should be called before context.setInput(Input.of(sourceConfig.getReferenceName(), new SourceInputFormatProvider( DataDrivenETLDBInputFormat.class, connectionConfigAccessor.getConfiguration()))) is called.

Otherwise error details provider information is not propagated to platform.

itsankit-google · 2024-12-05T09:39:39Z

Please squash the commits before merge.

psainics changed the title ~~Add MysqlErrorDetailsProvider~~ [PLUGIN-0000] ErrorDetailsProvider - MySql Source/Sink plugin Nov 20, 2024

psainics changed the title ~~[PLUGIN-0000] ErrorDetailsProvider - MySql Source/Sink plugin~~ [PLUGIN-1824] ErrorDetailsProvider - MySql Source/Sink plugin Nov 20, 2024

psainics marked this pull request as ready for review November 20, 2024 11:57

psainics self-assigned this Nov 20, 2024

psainics marked this pull request as draft November 20, 2024 11:58

psainics added the build label Nov 20, 2024

itsankit-google reviewed Nov 21, 2024

View reviewed changes

psainics force-pushed the fem/mysql branch 2 times, most recently from b755515 to 84432d6 Compare November 21, 2024 09:03

psainics requested a review from itsankit-google November 22, 2024 08:59

psainics marked this pull request as ready for review November 22, 2024 08:59

itsankit-google reviewed Dec 2, 2024

View reviewed changes

psainics requested a review from itsankit-google December 3, 2024 09:27

itsankit-google reviewed Dec 4, 2024

View reviewed changes

psainics requested review from AnkitCLI and itsankit-google December 4, 2024 06:49

itsankit-google approved these changes Dec 4, 2024

View reviewed changes

Add MysqlErrorDetailsProvider

26dc98e

psainics force-pushed the fem/mysql branch from ae2b573 to 26dc98e Compare December 5, 2024 13:00

psainics merged commit 23af441 into data-integrations:develop Dec 9, 2024
10 checks passed

psainics deleted the fem/mysql branch December 9, 2024 07:48

[PLUGIN-1824] ErrorDetailsProvider - MySql Source/Sink plugin #523

[PLUGIN-1824] ErrorDetailsProvider - MySql Source/Sink plugin #523

Uh oh!

Conversation

psainics commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ErrorDetailsProvider - MySql [Source|Sink] plugin

Description

Code change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsankit-google left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsankit-google Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsankit-google commented Dec 5, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

psainics commented Nov 20, 2024 •

edited

Loading

itsankit-google Dec 2, 2024 •

edited

Loading